Skip to content

Conversation

@aviatesk
Copy link
Member

@aviatesk aviatesk commented Jun 5, 2025

This optimizes the hash implementation added in
#452. Creating tuples with statically unknown types that may hold random values at runtime should be avoided for performance reasons. As a result, the allocation regression reported in #558 has been reduced to the previous level, although the time regression from hash calculation still remains.

@aviatesk aviatesk force-pushed the avi/optimize-hash branch 3 times, most recently from eb1ca9e to 457bca7 Compare June 5, 2025 15:19
This optimizes the `hash` implementation added in
#452. Creating tuples with statically
unknown types that may hold random values at runtime should be avoided
for performance reasons. As a result, the allocation regression
reported in #558 has been reduced to the
previous level, although the time regression from `hash` calculation
still remains.
@aviatesk aviatesk force-pushed the avi/optimize-hash branch from 457bca7 to f57460b Compare June 5, 2025 15:24
@aviatesk aviatesk merged commit eceaa39 into main Jun 7, 2025
36 checks passed
@aviatesk aviatesk deleted the avi/optimize-hash branch June 7, 2025 07:46
@Keno
Copy link
Member

Keno commented Jun 7, 2025

I don't think we really need to hash down the whole tree - the important thing is that equal green nodes have equal hashes (while not being too collide-y). I would propose we hash the length of the children and the (kinds/# of children) of the first child, but do not recurse to the second level.

c42f pushed a commit to JuliaLang/julia that referenced this pull request Oct 17, 2025
This optimizes the `hash` implementation added in
JuliaLang/JuliaSyntax.jl#452. Creating tuples with statically
unknown types that may hold random values at runtime should be avoided
for performance reasons. As a result, the allocation regression
reported in JuliaLang/JuliaSyntax.jl#558 has been reduced to the
previous level, although the time regression from `hash` calculation
still remains.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants